Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Split out downloadTestRepo from testRepoRoot in mill's own build #4179

Merged
merged 1 commit into from
Dec 25, 2024

Conversation

lihaoyi
Copy link
Member

@lihaoyi lihaoyi commented Dec 25, 2024

Fixes #4176. testRepoRoot.super always needs to re-evaluate, because it is a Task.Source. But if we split the downloadTestRepo call into a separate cached task that ensures the download is cached and can be re-used every time our testRepoRoot override needs to re-evaluate

@lihaoyi
Copy link
Member Author

lihaoyi commented Dec 25, 2024

Splitting out downloadTestRepo does mitigate the problem, but the underlying question remains why "example.thirdparty[gatling].testRepoRoot" seems to be an invalidation root when neither of its upstream tasks changed and it itself is not a source or input task

@lihaoyi lihaoyi marked this pull request as ready for review December 25, 2024 13:26
@lihaoyi
Copy link
Member Author

lihaoyi commented Dec 25, 2024

The problem appears to come from the fact that the preparatory input evaluation done in selective execution uses a subset of the tasks, resulting in the inputs being labelled differently during planning, such that the overridden example.thirdparty[gatling].testRepoRoot in the preparatory evaluation has the same label as the overridingexample.thirdparty[gatling].testRepoRoot in the main evaluation. This causes them to collide on disk and results in the latter unnecessarily invalidating since the hashes appear to have changed (since the other task stomped over the json files on disk)

This whole "tasks are assigned their final label during planning" thing is an endless source of bugs, and was originally introduced because we started using the ordering of tasks during planning to determine override ordering which is necessary to assign .super.Foo suffixes as part of the labels.

We probably should try to detect move override detection and label allocation to task instantiation time to avoid this footgun.

@lihaoyi lihaoyi merged commit 4f79aa1 into com-lihaoyi:main Dec 25, 2024
26 checks passed
@lefou lefou added this to the 0.12.5 milestone Dec 27, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move override segment suffixing from planning time to task instantiation time
2 participants